branch-4.1:[fix](iceberg)fix iceberg write special partition error. (#64225)#64339
Open
hubgeter wants to merge 1 commit into
Open
branch-4.1:[fix](iceberg)fix iceberg write special partition error. (#64225)#64339hubgeter wants to merge 1 commit into
hubgeter wants to merge 1 commit into
Conversation
…pache#64225) ### What problem does this PR solve? Issue Number: close #xxx Related PR: #xxx Problem Summary: ### Release note None ### Check List (For Author) - Test <!-- At least one of them must be included. --> - [ ] Regression test - [ ] Unit Test - [ ] Manual test (add detailed scripts or steps below) - [ ] No need to test or manual test. Explain why: - [ ] This is a refactor/code format and no logic has been changed. - [ ] Previous test can cover this change. - [ ] No code files have been changed. - [ ] Other reason <!-- Add your reason? --> - Behavior changed: - [ ] No. - [ ] Yes. <!-- Explain the behavior change --> - Does this need documentation? - [ ] No. - [ ] Yes. <!-- Add document PR link here. eg: apache/doris-website#1214 --> ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into -->
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 28656 ms |
Contributor
TPC-DS: Total hot run time: 169192 ms |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Automated review result: no blocking issues found.
Critical checkpoint conclusions:
- Goal/test: this replaces Hive-style escaping with Iceberg Java URLEncoder-compatible escaping for Iceberg partition paths. The BE unit test covers the encoder, and the external regression covers dynamic Iceberg writes with UTF-8 and special characters in partition values.
- Scope/focus: the change is small and focused: one helper, one writer call-site swap, and targeted tests.
- Concurrency/lifecycle/config/protocol: no new concurrency, lifecycle ownership, static initialization dependency, configuration, FE-BE protocol, or persisted metadata format was introduced.
- Parallel paths: dynamic, full static, and hybrid Iceberg partition write paths all route through the same _escape helper; I did not find another Iceberg data writer path that needs the same update.
- Data write correctness: file creation paths and committed original file paths are built from the same encoded partition path, while partition values in commit data remain unencoded as before.
- Testing/results: compile, BE UT, external regression, P0 regression, and coverage statuses are green in PR metadata. The GitHub macOS BE UT check failed before building because the runner used JDK 25 while the workflow requires JDK-17, so I did not treat that as a patch-related failure.
- Observability/performance: no additional observability appears necessary for this local path-encoding fix; the per-partition encoding cost is bounded and not a new heavy operation in a critical loop beyond existing partition dispatch.
- Security/threat model: this is not a security review and does not touch authentication, authorization, tenancy, or network trust boundaries.
- Existing review context: the provided inline review thread file contained no existing inline comments, so there was nothing to deduplicate.
- User focus: no additional user-provided review focus was supplied.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change. - [ ] No code files have been changed. - [ ] Other reason
Behavior changed:
Does this need documentation?
Yes.
Check List (For Reviewer who merge this PR)
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)